[Calendar] Remove promise based loading in favor of loading prop
#1829
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/mui-org/material-ui-pickers/qp2h2p173 |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| }) | ||
| .then(res => res.json()) | ||
| .then(({ daysToHighlight }) => { | ||
| setSelectedDays(daysToHighlight); |
There was a problem hiding this comment.
What if the component unmounts before the request resolves? I believe the set state call will warn/throw. The solution proposed in could be applied here too mui/mui-x#5 (comment)
There was a problem hiding this comment.
We are saving abort controller. So needs to make an effect that will abort request on unmount. thanks
There was a problem hiding this comment.
It's one way to solve this. I have no idea what's the best approach. I guess it's good enough, no need to look further 👍
There was a problem hiding this comment.
Interesting, but I think that this solution has one giant pitfall – it doesn't abort the request.
Probably if we want to support IE we must not use fetch and promises at all.
There was a problem hiding this comment.
Can requests be aborted? I mean, does it change something at the network layer?
There was a problem hiding this comment.
Actually request/response model cannot be aborted, but browsers can prevent some calculations on the responses if they are aborted (like processing low-level body and CORS)
Here network log when fast switching between months with request aborts:
Without:
This PR closes #1454
User facing changes
Breaking changes
Promisefrom theonMonthChangePropto render spinner in the component. From now on useloadingprop to renderLoadingComponentin the calendar.loadingIndicatortoLoadingComponentFeatures
loadingprop that will render loading placeholder while data is loading.CalendarSkeletoncomponent that can be used to achieve first-class UX experience for preloading data on month change: